Skip to content

Migrate tests to JUnit5#555

Merged
zkaoudi merged 7 commits into
apache:mainfrom
strangelookingnerd:migrate_to_junit5
Jun 3, 2025
Merged

Migrate tests to JUnit5#555
zkaoudi merged 7 commits into
apache:mainfrom
strangelookingnerd:migrate_to_junit5

Conversation

@strangelookingnerd
Copy link
Copy Markdown
Contributor

This PR aims to migrate all tests to JUnit5. Changes include:

  • Migrate annotations and imports
  • Migrate assertions
  • Remove public visibility for test classes and methods
  • Minor clean up

I am well aware that this is a huge changeset however I hope that there is still interest in this PR and it will be reviewed.
If there are any questions, please do not hesitate to ping me.

Why this is a good change:

  • JUnit 5 is the modern standard: It offers a cleaner and more powerful programming model, better extensibility, and improved support for modern Java features (like lambdas, streams, and optional parameters).
  • Improved test maintainability: JUnit 5’s more expressive annotations and lifecycle management make tests easier to read, write, and debug.
  • Enables use of modern extensions: Migrating paves the way to leverage powerful third-party extensions and tooling (e.g., parameterized tests, dynamic tests, conditional execution).

It is important to notice that this change should not alter the test logic, but bring the project in line with modern best practices and help keeping it future-proof.

* Migrate annotations and imports
* Migrate assertions
* Remove public visibility for test classes and methods
* Minor code cleanup
@zkaoudi
Copy link
Copy Markdown
Contributor

zkaoudi commented Apr 26, 2025

Hi @strangelookingnerd! Thank you for your contribution! This PR will take some time to be reviewed :)

@strangelookingnerd
Copy link
Copy Markdown
Contributor Author

Hi @strangelookingnerd! Thank you for your contribution! This PR will take some time to be reviewed :)

All good, I'll try my best to resolve merge conflicts in the meantime.

# Conflicts:
#	wayang-api/wayang-api-sql/src/test/java/org/apache/wayang/api/sql/SqlToWayangRelTest.java
@zkaoudi
Copy link
Copy Markdown
Contributor

zkaoudi commented May 19, 2025

Hi @strangelookingnerd, can you check and resolve the conflicts? Also some test is failing, if you could also check that it would be great.

# Conflicts:
#	pom.xml
#	wayang-api/wayang-api-sql/src/test/java/org/apache/wayang/api/sql/SqlToWayangRelTest.java
#	wayang-applications/pom.xml
#	wayang-tests-integration/src/test/java/org/apache/wayang/tests/SparkIntegrationIT.java
# Conflicts:
#	wayang-tests-integration/src/test/java/org/apache/wayang/tests/SparkIntegrationIT.java
@strangelookingnerd
Copy link
Copy Markdown
Contributor Author

Fixed merge conflicts.

@strangelookingnerd
Copy link
Copy Markdown
Contributor Author

Not really sure what is causing the test failures, any input would be appreciated.

2025-05-28T15:41:32.4051810Z [ERROR] Errors: 
2025-05-28T15:41:32.4053329Z [ERROR]   JavaIntegrationIT.testReadAndTransformAndWriteWithIllegalConfiguration1:98 » Wayang Could not find a single execution plan.
2025-05-28T15:41:32.4054527Z [ERROR]   SparkIntegrationIT.testReadAndTransformAndWriteWithIllegalConfiguration1:98 » Wayang Could not find a single execution plan.

@zkaoudi
Copy link
Copy Markdown
Contributor

zkaoudi commented Jun 3, 2025

I'm not familiar with the new junit api, but the tests where the new changes fail are the tests that are supposed to cause an exception.

Screenshot 2025-06-03 at 12 18 59

@strangelookingnerd
Copy link
Copy Markdown
Contributor Author

@zkaoudi Apparently I'm getting snowblind looking at my own changes 😁 I think I got it now.

@zkaoudi
Copy link
Copy Markdown
Contributor

zkaoudi commented Jun 3, 2025

Super! Everything works now! Thank you for the contribution :)

@zkaoudi zkaoudi merged commit eedd4f8 into apache:main Jun 3, 2025
4 checks passed
@strangelookingnerd strangelookingnerd deleted the migrate_to_junit5 branch June 3, 2025 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants